Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't NCPAINT our window, PAINT our window #1898

Merged
merged 7 commits into from
Jul 12, 2019

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Jul 9, 2019

Summary of the Pull Request

Fixes the round corners and lack of shadow on the window introduced in #929.
image

References

This introduces the problem discussed in #1897

PR Checklist

Detailed Description of the Pull Request / Additional comments

  • If we handle WM_NCPAINT, we don't get shadows. We also get the legacy rounded top window corners, because of some kernel insanity. The whole point of DwmExtendFrameIntoClientArea is that we can just PAINT the entire window, instead of trying to do NC_PAINT tricks. So, we need to move the entire NCPAINT we had into PAINT.
  • We also need to remove the margins we were setting. If we leave a top margin, the system caption buttons will try and draw into that area. Since we don't want those, we need to make sure the top margin is 0, so they have no space to draw into.
  • I also split out the DragSizeChanged handler from OnSize. I don't know why, but having it inside OnSize would cause the xaml island content to go black when moving the window from a secondary normal DPI display back to a high DPI primary display.

  Don't do anything in NCPAINT. If you do, you have to do everything. But the
  whole point of DwmExtendFrameIntoClientArea is to let you paint the NC area in
  your normal paint. So just do that dummy.

  * This doesn't transition across monitors.
  * This has a window style change I think is wrong.
  * I'm not sure the margins change is important.
  I'm not entirely sure why. But if we only update the titlebar drag region when
  that actually changes, it's a _lot_ smoother. I'm not super happy with the
  duplicated work in _UpdateDragRegion and OnSize, but checking this in in case
  I can't figure that out.
@DHowett-MSFT
Copy link
Contributor

Where are the resize handles here? Are they outside, on the shadow or inside, in the white border?

@zadjii-msft
Copy link
Member Author

Where are the resize handles here? Are they outside, on the shadow or inside, in the white border?

They are exactly on the border, unfortunately. No worse than before. I might be able to play with our NCHITTEST code - maybe we're clipping the resize handle area, though no promises.

@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. labels Jul 11, 2019
@DHowett-MSFT
Copy link
Contributor

Bad news!
When maximized with the button, the hole isn't painted.
image

@zadjii-msft
Copy link
Member Author

@DHowett-MSFT For the record, I fixed the titlebar maximize thing in 9040940 (what a sha)

Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love it.

@@ -113,8 +118,53 @@ void NonClientIslandWindow::OnSize(const UINT width, const UINT height)
_rootGrid.Arrange(finalRect);
}

// I'm not sure that HWND_BOTTOM is any different than HWND_TOP for us.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this comment is supposed to mean.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh mainly just that both those values do the same thing, which doesn't really make sense, so there's not really a reason one was picked over the other

@zadjii-msft zadjii-msft merged commit f4e02d8 into master Jul 12, 2019
@zadjii-msft zadjii-msft deleted the dev/migrie/f/1625-less-round-more-shadow branch July 12, 2019 19:46
mcpiroman pushed a commit to mcpiroman/terminal that referenced this pull request Jul 23, 2019
* This definitely works for getting shadow, pointy corners back

  Don't do anything in NCPAINT. If you do, you have to do everything. But the
  whole point of DwmExtendFrameIntoClientArea is to let you paint the NC area in
  your normal paint. So just do that dummy.

  * This doesn't transition across monitors.
  * This has a window style change I think is wrong.
  * I'm not sure the margins change is important.

* The window style was _not_ important

* Still getting a black xaml islands area (the HRGN) when we switch to high DPI

* I don't know if this affects anything.

* heyo this works.

  I'm not entirely sure why. But if we only update the titlebar drag region when
  that actually changes, it's a _lot_ smoother. I'm not super happy with the
  duplicated work in _UpdateDragRegion and OnSize, but checking this in in case
  I can't figure that out.

* Add more comments and cleanup

* Some PR nits, fix the titlebar painting on maximize
@ghost
Copy link

ghost commented Aug 3, 2019

🎉Windows Terminal Preview v0.3.2142.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We need to draw the whole non-client area ourselves
4 participants